Skip to content

Conversation

ChrisDenton
Copy link
Member

safe_remove_dir_all and safe_remove_file use canonicalize to workaround a MAX_PATH limitation. However, this has not been needed in a long time, since the standard library handles this situation itself.

I've kept safe_remove_file (without canonicalize) because it also returns Ok if the file is not found. While, safe_remove_file is only used twice, matching on the error kind is sufficiently verbose that maybe it's still worth it?

@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 14, 2025
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 15, 2025

📌 Commit 1d75783 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 15, 2025
…chenkov

Remove safe remove

`safe_remove_dir_all` and `safe_remove_file` use `canonicalize` to workaround a `MAX_PATH` limitation. However, this has not been needed in a long time, since the standard library handles this situation itself.

I've kept `safe_remove_file` (without `canonicalize`) because it also returns `Ok` if the file is not found. While, `safe_remove_file` is only used twice, matching on the error kind is sufficiently verbose that maybe it's still worth it?
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#138906 (Reject test executables when not supported by target)
 - rust-lang#139818 (Normalize ADT field in `find_tails_for_unsizing`)
 - rust-lang#139819 (Use `rust-cache` to speed-up `citool` compilation)
 - rust-lang#139824 (Remove safe remove)
 - rust-lang#139859 (CI: rename MacOS runner)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138455 (`librustdoc`: more `impl fmt::Display`)
 - rust-lang#139818 (Normalize ADT field in `find_tails_for_unsizing`)
 - rust-lang#139819 (Use `rust-cache` to speed-up `citool` compilation)
 - rust-lang#139824 (Remove safe remove)
 - rust-lang#139848 ( Reduce kw::Empty usage, part 5)
 - rust-lang#139859 (CI: rename MacOS runner)
 - rust-lang#139877 (Add warning comment to `Take::get_ref` and `Chain::get_ref`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8bb01ff into rust-lang:master Apr 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup merge of rust-lang#139824 - ChrisDenton:non-canonical, r=petrochenkov

Remove safe remove

`safe_remove_dir_all` and `safe_remove_file` use `canonicalize` to workaround a `MAX_PATH` limitation. However, this has not been needed in a long time, since the standard library handles this situation itself.

I've kept `safe_remove_file` (without `canonicalize`) because it also returns `Ok` if the file is not found. While, `safe_remove_file` is only used twice, matching on the error kind is sufficiently verbose that maybe it's still worth it?
@ChrisDenton ChrisDenton deleted the non-canonical branch April 16, 2025 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants